feat: stream container stderr in real-time instead of only on failure #621
Conversation
Container/process stderr is now streamed to logs continuously while the process runs, rather than only being printed when the connection fails. This helps with debugging container issues as they happen. - Uses io.MultiWriter to capture stderr to both a buffer and a pipe - Goroutine streams stderr lines to logs in real-time - Buffer is still available for error reporting on failure
There was a problem hiding this comment.
Pull request overview
This PR enhances debugging capabilities by streaming container stderr output to logs in real-time as the process runs, rather than only displaying it when the connection fails.
Changes:
- Implements real-time stderr streaming using io.MultiWriter to write to both a buffer and a pipe
- Launches a goroutine that continuously reads from the pipe and logs stderr lines as they arrive
- Maintains the existing buffer for error reporting on connection failures
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| logConn.Printf("[stderr] %s", line) | ||
| } | ||
| }() | ||
|
|
There was a problem hiding this comment.
The stderrPipeWriter is only closed in the error path (line 224) but not on the success path. When the connection succeeds and returns (line 274), the pipe writer remains open, which will cause the stderr streaming goroutine to hang indefinitely, waiting for more data. This creates a goroutine leak.
The pipe writer should be stored in the Connection struct and closed when Connection.Close() is called, or alternatively, the goroutine should monitor the context cancellation to know when to stop.
| // Ensure the stderr streaming goroutine terminates when the context is canceled | |
| go func() { | |
| <-ctx.Done() | |
| // Closing the reader will unblock the scanner and allow the goroutine to exit | |
| _ = stderrPipeReader.Close() | |
| }() |
| line := scanner.Text() | ||
| logger.LogInfo("backend", "[%s stderr] %s", command, line) | ||
| logConn.Printf("[stderr] %s", line) | ||
| } |
There was a problem hiding this comment.
The scanner may encounter errors (e.g., if the pipe is broken or a line is too long), but these errors are not checked or logged. After the scanning loop ends, scanner.Err() should be checked to detect and log any scanning errors. This is a common pattern seen elsewhere in the codebase (e.g., internal/cmd/root.go:439 and internal/config/validation_env.go, though the latter doesn't check scanner.Err() either).
| } | |
| } | |
| if err := scanner.Err(); err != nil { | |
| logger.LogErrorMd("backend", "Error reading MCP backend stderr: %v", err) | |
| logConn.Printf("Error reading MCP backend stderr: %v", err) | |
| } |
1 participant
Container/process stderr is now streamed to logs continuously while the process runs, rather than only being printed when the connection fails. This helps with debugging container issues as they happen.